Show purchase status in settings #426
Conversation
WalkthroughThe changes reorder Settings sections to add an unlockFullVersionSection and conditionally show it when Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-11-11T11:58:54.204ZApplied to files:
🧬 Code graph analysis (1)Cryptomator/Settings/SettingsViewModel.swift (3)
🔇 Additional comments (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Cryptomator/Settings/SettingsViewModel.swift(1 hunks)SharedResources/en.lproj/Localizable.strings(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Cryptomator/Settings/SettingsViewModel.swift (5)
Cryptomator/Common/Cells/ButtonCellViewModel.swift (1)
createDisclosureButton(19-21)Cryptomator/Settings/SettingsCoordinator.swift (3)
showAbout(31-35)showManageSubscriptions(85-97)showUnlockFullVersion(79-83)CryptomatorCommon/Sources/CryptomatorCommonCore/LocalizedString.swift (1)
getValue(12-22)Cryptomator/Purchase/IAPViewController.swift (1)
restorePurchase(277-290)Cryptomator/Purchase/PurchaseViewController.swift (1)
restorePurchase(55-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and test (freemium)
🔇 Additional comments (2)
SharedResources/en.lproj/Localizable.strings (1)
216-216: LGTM! Localization key follows conventions and uses consistent terminology.The new localization string is well-formed and appropriately placed within the Settings section. The key name "fullVersionStatus" clearly indicates its purpose, and "Full Version" matches the terminology used throughout the app.
Cryptomator/Settings/SettingsViewModel.swift (1)
79-84: Status cell implementation is correct.The localization key
"settings.fullVersionStatus"exists in the English localization file with the value "Full Version". The cell implementation is appropriate, usingBindableTableViewCellViewModelwithselectionStyle: .noneandaccessoryType: .checkmarkto correctly represent a non-interactive status indicator.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
Cryptomator/Settings/SettingsViewController.swift (1)
103-141: Simplify didSelectRow logic by handlingPurchaseStatusCellViewModelup frontFunctionally this works (purchase status row deselects and triggers
showUnlockFullVersion()), but you now:
- Call
dataSource?.itemIdentifier(for:)twice, and- Potentially deselect the same row twice for
PurchaseStatusCellViewModel.You can keep behavior while tightening the code by handling the purchase‑status case first and reusing the same item identifier, e.g.:
override func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { - let cellViewModel = dataSource?.itemIdentifier(for: indexPath) as? ButtonCellViewModel<SettingsButtonAction> + let item = dataSource?.itemIdentifier(for: indexPath) + + if item is PurchaseStatusCellViewModel { + tableView.deselectRow(at: indexPath, animated: true) + coordinator?.showUnlockFullVersion() + return + } + + let cellViewModel = item as? ButtonCellViewModel<SettingsButtonAction> @@ - switch cellViewModel?.action { + switch cellViewModel?.action { // existing cases… } - - if dataSource?.itemIdentifier(for: indexPath) is PurchaseStatusCellViewModel { - tableView.deselectRow(at: indexPath, animated: true) - coordinator?.showUnlockFullVersion() - } }Keeps the UX identical while reducing lookups and local complexity in this already large delegate method.
Cryptomator/Settings/PurchaseStatusCellViewModel.swift (1)
19-23: Consider using non-optional Bindable types for title and subtitle.The initializer accepts non-optional
Stringparameters fortitleandsubtitle(line 19), but wraps them inBindable<String?>(lines 16-17). Since these values are always present, usingBindable<String>instead would eliminate unnecessary optionality and make the code more type-safe.Apply this diff to use non-optional Bindable types:
- let title: Bindable<String?> - let subtitle: Bindable<String?> + let title: Bindable<String> + let subtitle: Bindable<String>Cryptomator/Settings/SettingsViewModel.swift (1)
103-118: Cache the DateFormatter to improve performance.The
DateFormatteris created on every access topurchaseStatusCellViewModel(lines 106-108). SinceDateFormattercreation is relatively expensive and this computed property is accessed whenever sections are rebuilt, consider caching the formatter as a static property or lazy var.Add a cached date formatter at the class level:
+ private static let expirationDateFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.dateStyle = .medium + formatter.timeStyle = .none + return formatter + }() + private var purchaseStatusCellViewModel: PurchaseStatusCellViewModel { let subtitle: String if let trialExpirationDate = cryptomatorSettings.trialExpirationDate, trialExpirationDate > Date() { - let dateFormatter = DateFormatter() - dateFormatter.dateStyle = .medium - dateFormatter.timeStyle = .none - subtitle = String(format: LocalizedString.getValue("settings.trial.expirationDate"), dateFormatter.string(from: trialExpirationDate)) + subtitle = String(format: LocalizedString.getValue("settings.trial.expirationDate"), Self.expirationDateFormatter.string(from: trialExpirationDate)) } else { subtitle = LocalizedString.getValue("settings.freeTier.subtitle") }Cryptomator/Settings/PurchaseStatusCell.swift (1)
30-66: LGTM! Layout implementation is clean and accessible.The Auto Layout setup is well-structured with:
- Proper use of layout margins
- Multi-line label support for dynamic type
- Appropriate spacing and sizing
One optional enhancement: consider setting
isAccessibilityElement = trueand providing a combined accessibility label that includes the icon meaning, title, and subtitle for VoiceOver users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Cryptomator.xcodeproj/project.pbxproj(4 hunks)Cryptomator/Settings/PurchaseStatusCell.swift(1 hunks)Cryptomator/Settings/PurchaseStatusCellViewModel.swift(1 hunks)Cryptomator/Settings/SettingsViewController.swift(1 hunks)Cryptomator/Settings/SettingsViewModel.swift(4 hunks)SharedResources/en.lproj/Localizable.strings(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: iammajid
Repo: cryptomator/ios PR: 426
File: Cryptomator/Settings/SettingsViewModel.swift:73-90
Timestamp: 2025-11-11T11:58:54.204Z
Learning: In SettingsViewModel.swift (Cryptomator iOS app), when displaying subscription/purchase status in the aboutSectionElements: subscriptions take precedence over lifetime licenses. If both hasRunningSubscription and fullVersionUnlocked are true (an edge case that shouldn't happen in practice), only the "Manage Subscriptions" button should be shown, not the "Full Version" status cell. This is intentional design behavior.
📚 Learning: 2025-11-11T11:58:54.204Z
Learnt from: iammajid
Repo: cryptomator/ios PR: 426
File: Cryptomator/Settings/SettingsViewModel.swift:73-90
Timestamp: 2025-11-11T11:58:54.204Z
Learning: In SettingsViewModel.swift (Cryptomator iOS app), when displaying subscription/purchase status in the aboutSectionElements: subscriptions take precedence over lifetime licenses. If both hasRunningSubscription and fullVersionUnlocked are true (an edge case that shouldn't happen in practice), only the "Manage Subscriptions" button should be shown, not the "Full Version" status cell. This is intentional design behavior.
Applied to files:
Cryptomator/Settings/PurchaseStatusCellViewModel.swiftCryptomator/Settings/SettingsViewController.swiftCryptomator/Settings/PurchaseStatusCell.swiftCryptomator/Settings/SettingsViewModel.swift
📚 Learning: 2024-10-10T15:32:49.838Z
Learnt from: tobihagemann
Repo: cryptomator/ios PR: 384
File: FileProviderExtensionUI/FileProviderCoordinator.swift:85-86
Timestamp: 2024-10-10T15:32:49.838Z
Learning: In `ReauthenticationViewController.swift`, the `coordinator` property is declared as `weak`, preventing retain cycles with `FileProviderCoordinator`.
Applied to files:
Cryptomator/Settings/SettingsViewController.swift
🧬 Code graph analysis (3)
Cryptomator/Settings/SettingsViewController.swift (2)
Cryptomator/Purchase/PurchaseViewController.swift (2)
tableView(31-41)tableView(43-53)Cryptomator/Settings/SettingsCoordinator.swift (1)
showUnlockFullVersion(79-83)
Cryptomator/Settings/PurchaseStatusCell.swift (1)
Cryptomator/Common/Combine/Publisher+OptionalAssign.swift (1)
assign(13-17)
Cryptomator/Settings/SettingsViewModel.swift (4)
Cryptomator/Common/TableViewModel.swift (1)
getFooterTitle(24-26)CryptomatorCommon/Sources/CryptomatorCommonCore/LocalizedString.swift (1)
getValue(12-22)Cryptomator/Purchase/PremiumManager.swift (3)
trialExpirationDate(50-52)trialExpirationDate(119-127)trialExpirationDate(168-170)Cryptomator/Common/Cells/ButtonCellViewModel.swift (1)
createDisclosureButton(19-21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and test (premium)
🔇 Additional comments (6)
Cryptomator.xcodeproj/project.pbxproj (1)
442-443: Purchase status cell + view model are correctly wired into the main app targetThe new
PurchaseStatusCell.swiftandPurchaseStatusCellViewModel.swiftare properly added to:
PBXFileReference(files exist in the Settings folder),PBXBuildFile(compile sources entries),- the
Settingsgroup hierarchy, and- the
Cryptomatortarget’sSourcesbuild phase.No config or target‑membership issues stand out.
Also applies to: 1063-1064, 2044-2045, 2792-2793
SharedResources/en.lproj/Localizable.strings (1)
216-218: LGTM! Clear and well-structured localization strings.The three new localization keys follow the existing naming convention and provide clear, user-friendly messages for the purchase status feature. The date format specifier on line 217 is correctly used.
Cryptomator/Settings/SettingsViewModel.swift (4)
52-77: LGTM! Dynamic section generation logic is clean and correct.The conditional inclusion of the purchase status section based on
hasFullAccessis well-implemented and aligns with the PR objectives.
79-82: LGTM! Footer title logic is correct.The implementation properly returns the footer text only for the about section when the user has full access.
84-86: LGTM! Clean abstraction for access checking.The
hasFullAccesscomputed property provides a clear, reusable check for premium access across the view model.
88-96: LGTM! Correctly removes the "Restore Purchase" button.The about section elements have been properly refactored to remove the "Restore Purchase" button, addressing the feedback from the previous review. The conditional inclusion of "Manage Subscriptions" is appropriate.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Cryptomator/Settings/SettingsViewModel.swift (1)
98-110: Purchase status cell cleanly covers trial vs free; consider reusing a formatter and confirmUIImageavailability.The
purchaseStatusCellViewModelnicely differentiates between an active trial and the free tier and wires the unlock action and subtitle style correctly. Two minor improvements:
- Reuse the
DateFormatterinstead of allocating one per call.You can move the formatter to a static property on
SettingsViewModeland reuse it here:class SettingsViewModel: TableViewModel<SettingsSection> { + private static let trialExpirationDateFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.dateStyle = .medium + formatter.timeStyle = .none + return formatter + }() @@ private var purchaseStatusCellViewModel: ButtonCellViewModel<SettingsButtonAction> { let detailTitle: String if let trialExpirationDate = cryptomatorSettings.trialExpirationDate, trialExpirationDate > Date() { - let dateFormatter = DateFormatter() - dateFormatter.dateStyle = .medium - dateFormatter.timeStyle = .none - detailTitle = String(format: LocalizedString.getValue("settings.trial.expirationDate"), dateFormatter.string(from: trialExpirationDate)) + detailTitle = String( + format: LocalizedString.getValue("settings.trial.expirationDate"), + Self.trialExpirationDateFormatter.string(from: trialExpirationDate) + ) } else { detailTitle = LocalizedString.getValue("settings.freeTier.subtitle") }
- Ensure
UIImageis available via imports.
UIImagelives in UIKit; if it isn’t already re‑exported by another imported module, you’ll need an explicitimport UIKitat the top of this file:-import Combine +import Combine +import UIKit
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
Cryptomator/AddVault/OpenExistingVault/OpenExistingVaultChooseFolderViewController.swift(1 hunks)Cryptomator/Common/Cells/BindableTableViewCellViewModel.swift(1 hunks)Cryptomator/Common/Cells/ButtonCellViewModel.swift(1 hunks)Cryptomator/Common/Cells/LoadingButtonCell.swift(1 hunks)Cryptomator/Common/Cells/SystemSymbolButtonCell.swift(1 hunks)Cryptomator/Common/ChooseFolder/CreateNewFolderViewController.swift(1 hunks)Cryptomator/Common/StaticUITableViewController.swift(1 hunks)Cryptomator/Settings/SettingsViewModel.swift(3 hunks)Cryptomator/VaultDetail/VaultDetailViewController.swift(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: iammajid
Repo: cryptomator/ios PR: 426
File: Cryptomator/Settings/SettingsViewModel.swift:73-90
Timestamp: 2025-11-11T11:58:54.204Z
Learning: In SettingsViewModel.swift (Cryptomator iOS app), when displaying subscription/purchase status in the aboutSectionElements: subscriptions take precedence over lifetime licenses. If both hasRunningSubscription and fullVersionUnlocked are true (an edge case that shouldn't happen in practice), only the "Manage Subscriptions" button should be shown, not the "Full Version" status cell. This is intentional design behavior.
📚 Learning: 2025-11-11T11:58:54.204Z
Learnt from: iammajid
Repo: cryptomator/ios PR: 426
File: Cryptomator/Settings/SettingsViewModel.swift:73-90
Timestamp: 2025-11-11T11:58:54.204Z
Learning: In SettingsViewModel.swift (Cryptomator iOS app), when displaying subscription/purchase status in the aboutSectionElements: subscriptions take precedence over lifetime licenses. If both hasRunningSubscription and fullVersionUnlocked are true (an edge case that shouldn't happen in practice), only the "Manage Subscriptions" button should be shown, not the "Full Version" status cell. This is intentional design behavior.
Applied to files:
Cryptomator/Common/Cells/ButtonCellViewModel.swiftCryptomator/Settings/SettingsViewModel.swift
🧬 Code graph analysis (2)
Cryptomator/Common/StaticUITableViewController.swift (1)
Cryptomator/VaultDetail/VaultDetailViewModel.swift (1)
cellViewModel(199-205)
Cryptomator/VaultDetail/VaultDetailViewController.swift (1)
Cryptomator/VaultDetail/VaultDetailViewModel.swift (1)
cellViewModel(199-205)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and test (premium)
🔇 Additional comments (9)
Cryptomator/Common/Cells/BindableTableViewCellViewModel.swift (1)
42-44: LGTM!The new
cellStyleproperty provides a clean foundation for the cell styling mechanism. The.defaultreturn value is appropriate for the base class and will be overridden by subclasses as needed.Cryptomator/Common/ChooseFolder/CreateNewFolderViewController.swift (1)
38-38: LGTM!The updated cell initialization correctly uses the new
cellStyleproperty from the view model, aligning with the cell styling mechanism introduced in this PR.Cryptomator/Common/Cells/ButtonCellViewModel.swift (1)
12-23: LGTM!The cell styling mechanism is well implemented:
- The
preferredCellStyleproperty with.value1default is appropriate for button cells that display both title and detail text- The updated initializer and factory method properly propagate the
cellStyleandimageparameters- The implementation is clean and consistent
Cryptomator/AddVault/OpenExistingVault/OpenExistingVaultChooseFolderViewController.swift (1)
48-48: LGTM!The cell initialization correctly adopts the new style-based pattern using
addVaultButtonViewModel.cellStyle.Cryptomator/VaultDetail/VaultDetailViewController.swift (1)
109-109: LGTM!The cell initialization properly uses the style-based pattern with
cellViewModel.cellStyle, consistent with the refactoring across the codebase.Cryptomator/Common/StaticUITableViewController.swift (1)
30-30: LGTM!The cell initialization correctly adopts the style-based pattern using
cellViewModel.cellStyle, consistent with the refactoring applied throughout the codebase.Cryptomator/Common/Cells/SystemSymbolButtonCell.swift (1)
12-12: Verify the base class change doesn't break functionality.Similar to
LoadingButtonCell, the superclass has been changed fromButtonTableViewCelltoTableViewCell. Ensure that no styling, initialization, or behavior fromButtonTableViewCellis required bySystemSymbolButtonCell.The verification script from the
LoadingButtonCellreview will also cover this change. Both cells should be tested together to ensure the base class migration is safe.Cryptomator/Common/Cells/LoadingButtonCell.swift (1)
11-11: Review comment is based on incorrect assumptions and should be ignored.The review comment assumes
ButtonTableViewCellexisted as the previous superclass, butButtonTableViewCelldoes not exist anywhere in the codebase. A comprehensive search across all Swift files, configuration files, and dependencies returned no results.The current implementation shows
LoadingButtonCellcorrectly inheriting fromTableViewCell, which provides all necessary functionality: theconfigure(with:)method, thesubscribersproperty, and access to UITableViewCell properties likedetailTextLabelandaccessoryView. No functionality is lost or at risk.Likely an incorrect or invalid review comment.
Cryptomator/Settings/SettingsViewModel.swift (1)
30-37: Section ordering and enum design are safe; no rawValue-based access patterns exist.The new
SettingsSectionenum reordering (introducingpurchaseStatusSection = 0and shifting others) is safe. Verification found no instances ofSettingsSection(rawValue:)or.rawValueaccess in the codebase. The architecture relies on positional array indexing (sections[section].id) rather than enum raw values, so reordering enum cases poses no risk.The conditional insertion of
purchaseStatusSection, the footer-only indication of full access for the about section, and theaboutSectionElementschange that enforces subscription precedence over lifetime unlocks are all sound and consistent with the intended design (per learnings).The optional defensive bounds check in
getFooterTitleremains a good practice but is not required:- override func getFooterTitle(for section: Int) -> String? { - guard sections[section].id == .aboutSection, hasFullAccess else { return nil } + override func getFooterTitle(for section: Int) -> String? { + guard section >= 0, section < sections.count else { return nil } + guard sections[section].id == .aboutSection, hasFullAccess else { return nil } return LocalizedString.getValue("settings.fullVersion.footer") }
Adds a visual indicator for users with a lifetime license in the Settings screen. Users with an active lifetime purchase now see a "Full Version" label with a checkmark, confirming their premium status. Additionally, the "Restore Purchase" button has been removed for subscription users as they already have active premium access.